Converting from APNG to WebP caused an error about timestamp being a float #6977
Conversation
|
Please can you include a test case that fails without the fix and passes with it? |
|
To demonstrate that this problem at least doesn't exist with all APNG files, using https://github.com/python-pillow/Pillow/blob/main/Tests/images/iss634.apng, the following code runs with Pillow 9.4.0 without an error. from PIL import Image
im = Image.open('iss634.apng')
im.save('out.webp') |
|
if you feel uncomfortable writing a test case, just posting a self-contained example here would still be helpful. |
for more information, see https://pre-commit.ci
|
This WIP test includes an image from https://signalstickers.com/pack/2ef10d945db55e0712773e18b24febe0 so it may not be properly redistributable as part of your suite, but it reproduces the issue when my patch is not applied |
|
Thanks. I see now that I missed the obvious fact that I needed to use If I replace your test with def test_float_duration(tmp_path):
temp_file = str(tmp_path / "temp.webp")
with Image.open("Tests/images/iss634.apng") as im:
assert im.info["duration"] == 70.0
im.save(temp_file, save_all=True)
with Image.open(temp_file) as reloaded:
reloaded.load()
assert reloaded.info["duration"] == 70does that sound good to you? It uses one of our existing test images. |
| @@ -285,7 +285,7 @@ def _save_all(im, fp, filename): | |||
| # Append the frame to the animation encoder | |||
| enc.add( | |||
| frame.tobytes("raw", rawmode), | |||
| timestamp, | |||
| int(timestamp), | |||
There was a problem hiding this comment.
What do you think about using round instead of int, here and below?
| @@ -305,7 +305,7 @@ def _save_all(im, fp, filename): | |||
| im.seek(cur_idx) | |||
|
|
|||
| # Force encoder to flush frames | |||
| enc.add(None, timestamp, 0, 0, "", lossless, quality, 0) | |||
| enc.add(None, int(timestamp), 0, 0, "", lossless, quality, 0) | |||
There was a problem hiding this comment.
| enc.add(None, int(timestamp), 0, 0, "", lossless, quality, 0) | |
| enc.add(None, round(timestamp), 0, 0, "", lossless, quality, 0) |
| @@ -285,7 +285,7 @@ def _save_all(im, fp, filename): | |||
| # Append the frame to the animation encoder | |||
| enc.add( | |||
| frame.tobytes("raw", rawmode), | |||
| timestamp, | |||
| int(timestamp), | |||
There was a problem hiding this comment.
| int(timestamp), | |
| round(timestamp), |
|
I've created PR #6996 with my two suggestions. |
|
Thanks for the PR, #6996 has been merged instead. |
Helps #7015
Changes proposed in this pull request: